Skip to content

Conversation

@pyguerder
Copy link
Contributor

I ran multiple tests using Blackfire. Memoizing these expensive calls reduces both RAM consumption & execution time.

This is an example with running on https://github.com/pyguerder/phpstan-propel2-example/blob/master/generated-classes/App/AnalyzeMe2.php only: https://blackfire.io/profiles/compare/7f25525d-0676-40b8-a809-a92692106be6/graph

And this is an example running on my main project (154 classes): https://blackfire.io/profiles/compare/7cc0ea82-3727-4a95-b948-112678bc0991/graph

@ondrejmirtes can you please have a look at this?

Thanks in advance, best regards

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this looks promising, thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to use a control structure (an if) here instead of just ?? on a single line. Like this:

if ($this->reflectionDocComment !== null) {
    $docComment = $this->reflectionDocComment;
} else {
    $docComment = $this->reflection->getDocComment();
    $this->reflectionDocComment = $docComment;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the same here.

This reduces both RAM consumption & execution time.
@pyguerder
Copy link
Contributor Author

@ondrejmirtes thanks for your feedback! Can you please look at it again?

I don't understand why some tests fail. I'm not sure it is related to my changes... Can you please advise?

@ondrejmirtes
Copy link
Member

Master is broken, it's not your fault 😊

@ondrejmirtes ondrejmirtes merged commit dfcb87e into phpstan:master Feb 26, 2021
@ondrejmirtes
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants